gha: enforce CHANGELOG.md entries for PRs #5047
Conversation
cyphar
force-pushed
the
gha-enforce-changelog
branch
5 times, most recently
from
5b81927 to
ec76631
Compare
Each release requires going through the list of PRs and trying to parse out what a reasonable changelog entry is (possibly ~6 months after the patch was last discussed and merged), which makes preparing release notes quite time-consuming. Given that the moment when the patch is merged is when the information about the imapct of a patch is most apparent to the author and reviewers, it seems prudent to require that every PR have a CHANGELOG.md change. Trivial patches can just add a dummy entry in the changelog (in future, we can expand this script to skip over commits that have some special tag to avoid the need for dummy entries). Signed-off-by: Aleksa Sarai <[email protected]>
cyphar
force-pushed
the
gha-enforce-changelog
branch
from
c8464c3 to
ba302df
Compare
cyphar
added
backport/1.2-todo
| opencontainers/cgroups#50) | ||
|
|
||
| ### Changed ### | ||
| - CI: All PRs now require a corresponding `CHANGELOG.md` change be included, |
There was a problem hiding this comment.
This is likely to just result in making the changelog lengthy and unreadable.
Also we will no longer be able to merge dependabot PRs.
There was a problem hiding this comment.
Maybe just migrate the changelog to wiki so that we can add changelog items without submitting PRs
There was a problem hiding this comment.
How will we make sure it gets updated on time? The current situation is that PRs get merged and then you need to figure out what to write for the changelog months later.
I would prefer to not have it be mandatory but it seems we are nowhere near consistent enough to enforce this through regular reviews.
For dependabot or minor PRs, I can adjust this CI job to allow-list PRs with special tags or by special committer emails...
One other option would be to enforce having a "for the changelog" section of the PR but then we would probably want to script collating them...
There was a problem hiding this comment.
I agree we have to add this somehow, and I guess adding the entry directly to CHANGELOG.md seems most straightforward. The downsides are
- there will definitely be more merge conflicts, especially for backports;
- dependabot PRs won't work (guess we can add an exception);
- the changelog style might end up less uniform;
- might make things more complicated for new contributors.
Perhaps we can try to enforce this manually for now?
There was a problem hiding this comment.
I've seen other projects avoid the merge conflict problem by using a directory and making a new file for each change. This has some extra overhead for everyone though, as the filenames are mean to be the PR number (submitters need to re-commit with the right PR number and we will still have to collate them).
For the dependabot PR issue we can make it a non-required CI job and so we can merge dependabot PRs but human PRs will have a failure so the submitter can proactively fix the issue.
Perhaps we can try to enforce this manually for now?
I think it'd be too easy to forget and having it be a CI job means that submitters can proactively fix it without needing us to remember to tell them.
|
FWIW, in moby we added a changelog section in the PR template so that we can extract those. Depending on the PR type, it's still optional, but perhaps variations thereof are possible (eg don't enforce if it's just a dependency bump); at least that prevents the merge-conflicts and allows tweaking changelog entries. |
|
@thaJeztah Ah, I didn't know it was that automated. That might be a good way of doing it, let me take a look... EDIT: Is the changelog collation also automated? |
|
It's definitely still a bit of work in progress, but perhaps good for inspiration / building blocks to come with something useful. I think many projects struggle with this; it's always hard to find the right balance between preparing the work as part of the PR (but tying to keep it low threshold), and still have some flexibility (also for those cases where PRs are superseded by follow ups, so not all changes may still be relevant). containerd uses a release tool that has some magic (but ultimately uses a toml file to collect change logs / allow preparing the release notes); it works, and also handles dependency updates, but perhaps could be combined with something like the above. |
4 participants
Each release requires going through the list of PRs and trying to parse
out what a reasonable changelog entry is (possibly ~6 months after the
patch was last discussed and merged), which makes preparing release
notes quite time-consuming.
Given that the moment when the patch is merged is when the information
about the imapct of a patch is most apparent to the author and
reviewers, it seems prudent to require that every PR have a CHANGELOG.md
change. Trivial patches can just add a dummy entry in the changelog (in
future, we can expand this script to skip over commits that have some
special tag to avoid the need for dummy entries).
Signed-off-by: Aleksa Sarai [email protected]